implement std.sha256 #607
implement std.sha256 #607
Conversation
* explicitly load git_repository rule * load googletest as a git_repository * stop using bind (see bazelbuild/bazel#1952)
| @@ -869,7 +869,12 @@ class Interpreter { | |||
| builtins["extVar"] = &Interpreter::builtinExtVar; | |||
| builtins["primitiveEquals"] = &Interpreter::builtinPrimitiveEquals; | |||
| builtins["native"] = &Interpreter::builtinNative; | |||
| #ifndef DISABLE_INSECURE_HASH | |||
| builtins["md5"] = &Interpreter::builtinMd5; | |||
There was a problem hiding this comment.
Any reason to leave the old one at all? Why not just always use OpenSSL version if we depend on it anyway?
There was a problem hiding this comment.
Backwards compatibility. We don't want to force people to redeploy their entire infrastructure because they were using md5 to generate unique names for things.
|
Thanks, adding more standard hash functions is a good thing. Unfortunately, going from 0 external dependencies to one (two with absl) is a pretty inconvenient. We need to support the users who don't use Bazel. Are there any good header-only hash function libs? That would be much more convenient here. However, OpenSSL is going to be available on pretty much any system anyway and it provides us with all the crypto we may need. I'm not aware of any other reasonable option, so we probably should use it. But then we need to make it work with a regular Makefile, add dependency information to README and make the new requirement very clear during the release. I don't think that I wonder what @sparkprime thinks about this. |
|
The only reason Mike did this is that someone internally wanted us to provide an alternative to MD5 in case anyone felt tempted to use MD5 in a context where crytographic hashing was needed. Personally I'm not really convinced it's worthwhile since this is a config language. As for absl dependency, I'm sure we can write our own absl::BytesToHexString :) There are already build problems on windows and mac and stuff like this will probably exacerbate them. Maybe we should pull off the bandaid and start adding stuff to go that's not in C++? |
|
If we will go forward with parseYaml we will probably want to add shared library dependencies anyway, so adding openSSL should't change that much. |
Adds a dep on openssl and absl. I'm not sure how problematic that is.